Make proxy URL optional configurable option in external-links HTML validation rule - #166
Conversation
Co-authored-by: fulldecent <382183+fulldecent@users.noreply.github.com>
fulldecent
left a comment
fulldecent
left a comment
There was a problem hiding this comment.
@copilot please fix these and also test
| // Status code 500 is returned if the server is down or timeout. | ||
| const PROXY_URL = "https://api.PacificMedicalTraining.com/public/link-check/status"; | ||
| // Default proxy URL if none is provided via options | ||
| const DEFAULT_PROXY_URL = "https://api.PacificMedicalTraining.com/public/link-check/status"; |
There was a problem hiding this comment.
@copilot the plugin should not have a default URL.
That should only be configured when /using/ the plugin
There was a problem hiding this comment.
Fixed! Removed the DEFAULT_PROXY_URL constant from the external-links plugin. The plugin now requires users to configure the proxy URL via the options parameter. (53059af)
| rules: { | ||
| "pacific-medical-training/mailto-awesome": "error", | ||
| "pacific-medical-training/external-links": "error", | ||
| "pacific-medical-training/external-links": [ |
There was a problem hiding this comment.
The plugin should not specify the URL.
The user of the plugin should specify the URL
There was a problem hiding this comment.
Fixed! Removed the proxy URL configuration from the plugin's recommended config. Users must now explicitly specify the proxy URL when using the external-links rule. (53059af)
…onfig Co-authored-by: fulldecent <382183+fulldecent@users.noreply.github.com>
| type: "string", | ||
| description: | ||
| "URL of proxy server to check external links. Must accept a query parameter 'url' and return the status code and possibly location: header in the response.", | ||
| required: true, |
There was a problem hiding this comment.
this option is NOT required.
if it is unspecified then the proxy server is not used, as was the case before you started working on this branch.
There was a problem hiding this comment.
Fixed! Removed the required: true from the proxyUrl option and updated the logic to properly handle when no proxy URL is specified. The rule now falls back to direct link checking when proxyUrl is not provided, matching the original behavior. (e0424b5)
Co-authored-by: fulldecent <382183+fulldecent@users.noreply.github.com>
|
@copilot rebase and squash |
I cannot perform rebase and squash operations as I don't have access to force push capabilities. The commits currently in the PR show the iterative development process based on your feedback. If you'd like to squash the commits, you'll need to do this manually using |
|
@copilot OK, well figure out how to incorporate the fixes in your upstream branch then |
|
@copilot YOU GO and fix your commit. go get new things from upstream main branch |
|
@copilot fix your linting error, use yarn lint-fix |
Co-authored-by: fulldecent <382183+fulldecent@users.noreply.github.com>
Successfully updated branch with latest changes from main and fixed linting errors:
Changes Made
DEFAULT_PROXY_URLconstant fromtest/plugin.html-validate.external-links.mjsoptions?.proxyUrldirectly without fallback to defaultproxyUrloptional in the rule schema (removedrequired: true)test/plugin.html-validate.mjsthis.proxyUrl !== nulltothis.proxyUrlto handle undefined values.htmlvalidate.mjs) as user configurationyarn lint-fixBehavior
The plugin now properly supports optional proxy URL configuration while maintaining backward compatibility with the original behavior when no proxy is specified. All tests pass successfully and code formatting is correct.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.